Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Websocket CPU efficiency improvements. #711

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

amcguier
Copy link
Contributor

@amcguier amcguier commented Oct 9, 2021

I see high cpu usage using the fsdocs watch command in current
release version. It appears that the loop inside the socket is running
constantly since it never yields to receive socket messages from the
client, causing the process to use 100% of available CPU. Also, since
the reload creates a new socket connection (and suave creates a new
handler) this compounds since a new infinite loop is created after
every reload.

I've replaced the mutable boolean with a ManualResetEvent which all
connected sockets block on until a reload is called for. Once the
socket connection is closed the old handler exits. I chose
ManualResetEvent over AutoResetEvent because we want all connected
sockets to restart at the same time, rather than individually, since
someone might have multiple tabs open.

I see high cpu usage using the `fsdocs watch` command in current
release version. It appears that the loop inside the socket is running
constantly since it never yields to receive socket messages from the
client, causing the process to use 100% of available CPU. Also, since
the reload creates a new socket connection (and suave creates a new
handler) this compounds since a new infinite loop is created after
every reload.

I've replaced the mutable boolean with a ManualResetEvent which all
connected sockets block on until a reload is called for. Once the
socket connection is closed the handler exits. I chose
ManualResetEvent over AutoResetEvent because we want all connected
sockets to restart at the same time, rather than individually, since
someone might have multiple tabs open.
@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2021

@amcguier It looks good. Could you outline the manual testing you did? Thanks

@amcguier
Copy link
Contributor Author

amcguier commented Oct 11, 2021

I've currently only tested under OSX.

Thus far I've:

  1. Done basic "do pages reload on save?" for a few projects.
  2. Verified that if I have multiple tabs open concurrently (or multiple browsers) on a project they all reload
  3. Verified that if I disconnect client side (by killing a tab) nothing unexpected happens, and hot reload works when I open another tab and reconnect.
  4. Some very precursory profiling with dotTrace before/after my change to verify what I was seeing in top/activity monitor

If folk are happy with the general approach here, I could repeat those tests on a windows/linux VM if needed.

@dsyme
Copy link
Contributor

dsyme commented Oct 11, 2021

@amcguier That's ok, it's enough - thank you!

@dsyme dsyme merged commit 816763f into fsprojects:master Oct 11, 2021
@dsyme
Copy link
Contributor

dsyme commented Oct 11, 2021

11.4.4 is in the process of being released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants